Skip to content

Partially stabilize LoongArch target features #135015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 11, 2025
Merged

Partially stabilize LoongArch target features #135015

merged 2 commits into from
May 11, 2025
+9 −13

Conversation

heiher
Copy link
Contributor

@heiher heiher commented Jan 2, 2025 •
edited
Loading

Stabilization PR for the LoongArch target features. This PR stabilizes some of the target features tracked by #44839.

Specifically, this PR stabilizes the following target features:

  • f
  • d
  • frecipe
  • lasx
  • lbt
  • lsx
  • lvz

Docs PR: rust-lang/reference#1707

r? @Amanieu

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 2, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 2, 2025

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@heiher heiher force-pushed the stabilize-loongarch-target-features branch from cb6584c to 29c94f5 Compare January 2, 2025 07:35
@jieyouxu jieyouxu added the O-loongarch Target: LoongArch (LA32R, LA32S, LA64) label Jan 2, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@RalfJung
Copy link
Member

RalfJung commented Jan 2, 2025 •
edited
Loading

If "d" and "f" work anything like the RISC-V features of the same name, they should not be stabilized yet. We need to figure out #116344 and #131799 for LoongArch first.

Does any of the other features ever have any effect on calling conventions or ABI?

@jieyouxu jieyouxu added the A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. label Jan 2, 2025
@heiher
Copy link
Contributor Author

heiher commented Jan 2, 2025

If "d" and "f" work anything like the RISC-V features of the same name, they should not be stabilized yet. We need to figure out #116344 and #131799 for LoongArch first.

Does any of the other features ever have any effect on calling conventions or ABI?

Thank you! Other than f and d, there are no other target features affecting the calling convention and ABI.

@heiher heiher changed the title Stabilize LoongArch target features Partially stabilize LoongArch target features Jan 2, 2025
@heiher heiher force-pushed the stabilize-loongarch-target-features branch from 29c94f5 to 4c59f0d Compare January 2, 2025 14:16
@ehuss
Copy link
Contributor

ehuss commented Jan 2, 2025

This will need a PR for documentation at https://github.com/rust-lang/reference/blob/master/src/attributes/codegen.md, and a lang (+whoever?) FCP.

Amanieu
Amanieu reviewed Jan 2, 2025
View reviewed changes
compiler/rustc_target/src/target_features.rs Outdated
Comment on lines 773 to 774
("relax", STABLE, &[]),
("ual", STABLE, &[]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • relax refers to linker relaxation support and may not be appropriate to expose as a target feature.
  • ual refers to support for unaligned memory access, similar to strict-align on ARM targets. It's probably fine to stabilize as-is, but we may want to double-check with other targets if we want to have a consistent feature name between all of them (alternatively it's also fine to make the feature target-specific).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also to be clear, ual can only affect how unaligned accesses are codegen'd, and nothing else. See this recent discussion on Zulip.

If we have target feature docs somewhere, they should clarify hat unaligned accesses are still UB except when performed via {read,write}_unaligned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with skipping these two target features and stabilizing them at a more appropriate time.

@Amanieu
Copy link
Member

Amanieu commented Jan 2, 2025

r? lang

@rustbot rustbot added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 2, 2025
@rustbot rustbot assigned joshtriplett and unassigned Amanieu Jan 2, 2025
@heiher heiher mentioned this pull request Jan 3, 2025
Open
@heiher
Copy link
Contributor Author

heiher commented Jan 3, 2025

This will need a PR for documentation at https://github.com/rust-lang/reference/blob/master/src/attributes/codegen.md, and a lang (+whoever?) FCP.

PR: rust-lang/reference#1707

@heiher heiher force-pushed the stabilize-loongarch-target-features branch from 4c59f0d to 71e17a0 Compare January 3, 2025 07:10
@bors
Copy link
Collaborator

bors commented Jan 6, 2025

☔ The latest upstream changes (presumably #134794) made this pull request unmergeable. Please resolve the merge conflicts.

@heiher heiher force-pushed the stabilize-loongarch-target-features branch from 71e17a0 to 49b6947 Compare January 6, 2025 06:56
@traviscross traviscross added the I-lang-nominated Nominated for discussion during a lang team meeting. label Jan 7, 2025
@bors
Copy link
Collaborator

bors commented Mar 15, 2025

☔ The latest upstream changes (presumably #138523) made this pull request unmergeable. Please resolve the merge conflicts.

@Amanieu Amanieu mentioned this pull request Mar 16, 2025
Open
@heiher heiher force-pushed the stabilize-loongarch-target-features branch from 49b6947 to f4c604d Compare March 18, 2025 09:48
@heiher heiher requested a review from Amanieu March 18, 2025 11:07
@heiher heiher force-pushed the stabilize-loongarch-target-features branch from f4c604d to 7cbc101 Compare March 19, 2025 07:20
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 8, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request May 9, 2025
@jhpratt
Rollup merge of rust-lang#135015 - heiher:stabilize-loongarch-target-…
efdcf91 
…features, r=Amanieu

Partially stabilize LoongArch target features

Stabilization PR for the LoongArch target features. This PR stabilizes some of the target features tracked by rust-lang#44839.

Specifically, this PR stabilizes the following target features:

* f
* d
* frecipe
* lasx
* lbt
* lsx
* lvz

Docs PR: rust-lang/reference#1707

r? `@Amanieu`
@jhpratt jhpratt mentioned this pull request May 9, 2025
Closed
@jhpratt
Copy link
Member

jhpratt commented May 9, 2025

@bors r-

#140835 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 9, 2025
@heiher heiher force-pushed the stabilize-loongarch-target-features branch from 1f2536f to 4a662c2 Compare May 9, 2025 03:26
@heiher
Copy link
Contributor Author

heiher commented May 9, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 9, 2025
@tgross35
Copy link
Contributor

tgross35 commented May 9, 2025

@bors r=Amanieu

@bors
Copy link
Collaborator

bors commented May 9, 2025

📌 Commit 4a662c2 has been approved by Amanieu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 9, 2025
@bors
Copy link
Collaborator

bors commented May 9, 2025

⌛ Testing commit 4a662c2 with merge 5437b87...

bors added a commit to rust-lang-ci/rust that referenced this pull request May 9, 2025
@bors
Auto merge of rust-lang#135015 - heiher:stabilize-loongarch-target-fe…
5437b87 
…atures, r=Amanieu

Partially stabilize LoongArch target features

Stabilization PR for the LoongArch target features. This PR stabilizes some of the target features tracked by rust-lang#44839.

Specifically, this PR stabilizes the following target features:

* f
* d
* frecipe
* lasx
* lbt
* lsx
* lvz

Docs PR: rust-lang/reference#1707

r? `@Amanieu`
@pietroalbini
Copy link
Member

@bors retry
Yield priority to the stable release.

Zalathar added a commit to Zalathar/rust that referenced this pull request May 10, 2025
@Zalathar
Rollup merge of rust-lang#135015 - heiher:stabilize-loongarch-target-…
2a20aa0 
…features, r=Amanieu

Partially stabilize LoongArch target features

Stabilization PR for the LoongArch target features. This PR stabilizes some of the target features tracked by rust-lang#44839.

Specifically, this PR stabilizes the following target features:

* f
* d
* frecipe
* lasx
* lbt
* lsx
* lvz

Docs PR: rust-lang/reference#1707

r? `@Amanieu`
@Zalathar Zalathar mentioned this pull request May 10, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2025
@bors
Auto merge of rust-lang#140885 - Zalathar:rollup-cg3dewe, r=Zalathar
84cff42 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#129334 (Implement (part of) ACP 429: add `DerefMut` to `Lazy[Cell/Lock]`)
 - rust-lang#135015 (Partially stabilize LoongArch target features)
 - rust-lang#138736 (Sanitizers target modificators)
 - rust-lang#139562 (rustdoc: add a handle that makes sidebar resizing more obvious)
 - rust-lang#140151 (remove intrinsics::drop_in_place)
 - rust-lang#140660 (remove 'unordered' atomic intrinsics)
 - rust-lang#140783 (Update documentation of OnceLock::get_or_init.)
 - rust-lang#140789 (Update hermit-abi to 0.5.1)
 - rust-lang#140792 (Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations)
 - rust-lang#140862 (Enable non-leaf Frame Pointers for Arm64EC Windows)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Collaborator

bors commented May 11, 2025

⌛ Testing commit 4a662c2 with merge 9a7e19f...

@bors
Copy link
Collaborator

bors commented May 11, 2025

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 9a7e19f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 11, 2025
@bors bors merged commit 9a7e19f into rust-lang:master May 11, 2025
7 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 11, 2025
@github-actions GitHub Actions
Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing fd6626d (parent) -> 9a7e19f (this PR)

Test differences

No test diffs found

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 9a7e19f2b66c472811b4bff02695c7a03f3f4151 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-x86_64-apple: 7840.2s -> 9767.0s (24.6%)
  2. x86_64-apple-1: 8479.7s -> 7022.4s (-17.2%)
  3. dist-arm-linux: 4536.7s -> 5228.6s (15.2%)
  4. x86_64-rust-for-linux: 3030.5s -> 2638.6s (-12.9%)
  5. aarch64-apple: 4040.5s -> 3633.2s (-10.1%)
  6. dist-various-1: 4392.5s -> 4709.8s (7.2%)
  7. x86_64-apple-2: 5355.7s -> 4978.0s (-7.1%)
  8. dist-aarch64-apple: 4663.2s -> 4887.9s (4.8%)
  9. arm-android: 5535.0s -> 5272.0s (-4.8%)
  10. dist-apple-various: 6794.9s -> 6472.9s (-4.7%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9a7e19f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -0.5%, secondary -2.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 1.1% [1.1%, 1.1%] 1
Regressions ❌
(secondary)		 2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)		 -0.6% [-1.5%, -0.4%] 11
Improvements ✅
(secondary)		 -4.7% [-5.1%, -4.3%] 2
All ❌✅ (primary) -0.5% [-1.5%, 1.1%] 12

Cycles

Results (primary -0.6%, secondary 0.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 3.0% [3.0%, 3.0%] 1
Improvements ✅
(primary)		 -0.6% [-0.7%, -0.5%] 3
Improvements ✅
(secondary)		 -2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) -0.6% [-0.7%, -0.5%] 3

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 772.163s -> 771.266s (-0.12%)
Artifact size: 365.40 MiB -> 365.36 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

@Amanieu Amanieu Awaiting requested review from Amanieu

Assignees

@joshtriplett joshtriplett

Labels
A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. merged-by-bors This PR was explicitly merged by bors. O-loongarch Target: LoongArch (LA32R, LA32S, LA64) relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.